Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add initial container definitions data source. #5862

Closed

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Sep 13, 2018

This is an early WIP that I'm submitting for feedback--don't use it yet! I would like to define ecs container definitions in hcl directly, without writing templated json, much like https://www.terraform.io/docs/providers/aws/d/iam_policy_document.html for iam policies. This new resource allows users to define container definitions like so:

data "aws_ecs_container_definitions" "test" {
  container_definitions {
    name = "container1"
    image = "image1"
    port_mappings {
      container_port = 5000
    }
  }
  container_definitions {
    name = "container2"
    image = "image2"
  }
}

output "container_json" {
  value = "${data.aws_ecs_container_definitions.test.json}"
}

If the use case makes sense, I'll flesh this out, write tests, make the code nicer, etc. WDYT @bflad ?

Allow the provider to manage container definitions json without writing
templated inline json or a separate json template file.
@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Sep 13, 2018
@bflad bflad added new-data-source Introduces a new data source. service/ecs Issues and PRs that pertain to the ecs service. labels Sep 13, 2018
@jmcarp
Copy link
Contributor Author

jmcarp commented Sep 13, 2018

@bflad I see you tagged this--thanks! Let me know if you'd be open to the feature, and I'll clean up the patch if so.

@bflad
Copy link
Contributor

bflad commented Sep 28, 2018

Hey @jmcarp 👋 I think in principal this is a good idea, but it might be helpful to see a design sketch of the planned configuration options/usage laid out and discussed in an issue first. There was a similar ask in #3153 (comment), but this is deserving of its own feature request for "Defining ECS Container Definitions in HCL". (I'll likely close out the YAML issue anyways since its not supported by the ECS API and the AWS provider should not concern itself with YAML -> JSON conversion.)

HCL with configuration validation and converting to JSON is a good start, but I think we'll want to look at the problem more holistically as well, e.g. how do I easily add container definition X to Y container definitions?

# Example design sketch - ignoring current aws_ecs_container_definition data source implementation
# Arguably, the existing aws_ecs_container_definition should be migrated to something like aws_ecs_task_definition_container_definition (what a gross name)
# Or preferrably, part of the aws_ecs_task_definition data source in a new container_definitions map with name as the key

data "aws_ecs_container_definition" "monitoring" {
  # name, image, etc.
}

data "aws_ecs_container_definition" "application1" {
  # name, image, etc.
}

data "aws_ecs_container_definition" "application2" {
  # name, image, etc.
}

data "aws_ecs_container_definitions" "service1" {
  container_definitions = [
    "${data.aws_ecs_container_definitions.monitoring.json}",
    "${data.aws_ecs_container_definitions.application1.json}",
  ]
}

data "aws_ecs_container_definitions" "service2" {
  container_definitions = [
    "${data.aws_ecs_container_definitions.monitoring.json}",
    "${data.aws_ecs_container_definitions.application2.json}",
  ]
}

Let's design and discuss this further in the new issue before trying to continue with any implementation. 👍

@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Sep 28, 2018
@bflad
Copy link
Contributor

bflad commented Jun 21, 2019

As mentioned in #5862 (comment) lets work towards a design proposal and then we can consider an appropriate implementation. 👍

That said, Terraform 0.12 does include a new jsonencode() function, which likely alleviates some of the need for a new data source in general except for validation purposes. 😄

@bflad bflad closed this Jun 21, 2019
@steveh
Copy link
Contributor

steveh commented Jun 22, 2019

This would still be very useful in order to ignore_changes to only select fields, e.g. if you want to deploy a new version of an image outside of Terraform.

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jun 22, 2019
@ghost
Copy link

ghost commented Nov 3, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-data-source Introduces a new data source. service/ecs Issues and PRs that pertain to the ecs service. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants